-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Release Best Practices Guide #149
base: main
Are you sure you want to change the base?
Conversation
@mike-gangl - would this help Unity make releases across many repositories? Let me know what you think. |
Standardized release template for SLIM as described in #149
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Token handling is probably the serious issue but see also my other comments which you can take or leave 😀
2.1 Download our script | ||
- Copy/download our script to your local machine | ||
``` | ||
curl -o gh_aggregate_release_notes.py https://raw.githubusercontent.com/NASA-AMMOS/slim/issue-97/docs/guides/documentation/releases/gh_aggregate_release_notes.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personal taste: in documentation like this, I always prefer the --long
format command-line arguments as they feel more "documentary" 😁 , so -o
→ --output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remember to switch that link to the main branch before releasing. I only mention it here because it is something that often slips through the cracks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @nutjob4life - fixed in latest commit.
@jpl-jengelke - will keep this comment unresolved so I don't forget!
|
||
2.2 Create a configuration file with your project's release information. Save the file with extension `.yml` A sample is provided below: | ||
``` | ||
github_token: <INSERT YOUR GH PERSONAL TOKEN> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use a brief hint here on how to get a token—maybe down on the FAQ?
In retrospect, I think this is awfully risky. Your token could afford a lot of permissions unless it's carefully scoped—and putting it into a file makes it too easy to check into version control (yay detect-secrets!) Maybe it should be an environment variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comment is to link to the GH Actions documentation directly -- I wouldn't put "" here, but instead I would make it explicit with ${{ secrets.GITHUB_TOKEN }}
. Then in 2.2
, something like, "Create a release notes template (.yml
) that's authorized to publish your project."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's OK because "secrets.GITHUB_TOKEN" is mnemonically explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nutjob4life - I was debating between an environment variable and a file. I felt that a file could be kept secure on a system with permission control, whereas the environment variable approach keeps a credential on the command-line history. Though I see the risk of committing this file and your point! Are these our only two choices, I'm curious about best practices here.
@jpl-jengelke - agreed to linking to GH directly and letting their docs handle the UI specifics.
@@ -0,0 +1,99 @@ | |||
import requests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script should probably include some comments at the top about dependencies. For example, it does import requests
so needs a Python installation (or virtual environment) with Requests installed. Bonus points for mentioning a version number of requests
too 👍 (same for tqdm
, yaml
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Echoes my comment above... Also, Python 3+.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also may want to add a little try-except error catching to explain to a casual user what's wrong. What if the template itself is munged up? That can be hard to catch.
Some minimal logging (even just print statements) may be helpful, too.
if response.status_code == 200: | ||
return response.json().get("body", "") | ||
else: | ||
print(f"Error fetching release notes for {url}: {response.status_code}. Retrying...") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe move the retry_num += 1
up and add an if retry_num < max_retries
to condition the sleep? The "edge" case is that GitHub gives non-200 every single time, but we end up doing a final sleep for 2 × 2⁴ + [0.0..1.0] seconds—up to 33 seconds—to then just abort without a final attempt.
In other words, change from attempt…sleep…attempt…sleep…attempt…sleep…attempt…sleep…abort
→ attempt…sleep…attempt…sleep…attempt…sleep…attempt…abort
🤷
Not a huge deal but I've been known to be rather pedantic 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me look into this thanks @nutjob4life
print("Failed to read configuration.") | ||
return | ||
|
||
github_token = config.get("github_token") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above about putting the token into the .yml
(or .yaml
where I come from 😁) file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe some changes, but specifically with the token stuff, would be good.
2.1 Download our script | ||
- Copy/download our script to your local machine | ||
``` | ||
curl -o gh_aggregate_release_notes.py https://raw.githubusercontent.com/NASA-AMMOS/slim/issue-97/docs/guides/documentation/releases/gh_aggregate_release_notes.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remember to switch that link to the main branch before releasing. I only mention it here because it is something that often slips through the cracks.
|
||
2.2 Create a configuration file with your project's release information. Save the file with extension `.yml` A sample is provided below: | ||
``` | ||
github_token: <INSERT YOUR GH PERSONAL TOKEN> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comment is to link to the GH Actions documentation directly -- I wouldn't put "" here, but instead I would make it explicit with ${{ secrets.GITHUB_TOKEN }}
. Then in 2.2
, something like, "Create a release notes template (.yml
) that's authorized to publish your project."
|
||
2.2 Create a configuration file with your project's release information. Save the file with extension `.yml` A sample is provided below: | ||
``` | ||
github_token: <INSERT YOUR GH PERSONAL TOKEN> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's OK because "secrets.GITHUB_TOKEN" is mnemonically explicit.
2.3 Run the script to generate aggregated release notes | ||
``` | ||
$ python gh_aggregate_release_notes.py your_configuration_file.yml | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I like the concept of building release notes based off the GH API and the script looks light enough. When I first read about this I though of using GH's built-in facilities to just copy it. Could it all be done in an action file without a separate Python component? What if the actual release notes stored in the 'releases' section of the repository are used or the macros that create them there are reused? I think if GH products are reused then GH handles maintenance, the notes are uniform and the maintenance footprint is reduced. (I can see Python versions and module releases being pesky as time goes forward, plus shouldn't there be a dependencies file (requirements.txt)?
@@ -0,0 +1,99 @@ | |||
import requests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Echoes my comment above... Also, Python 3+.
} | ||
|
||
max_retries = 5 | ||
retry_num = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use Tenacity here to automate retries over the function. It would also handle the manifest other potential exceptions that could occur.
@@ -0,0 +1,99 @@ | |||
import requests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also may want to add a little try-except error catching to explain to a casual user what's wrong. What if the template itself is munged up? That can be hard to catch.
Some minimal logging (even just print statements) may be helpful, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having a YAML-based template, which is very pedantic on format, may be a source of issues for casual users who aren't very pedantic with spacing, etc.
1. **Customize Release Notes**: | ||
- Create a `.github/release.yml` file in your repository. | ||
- Download our [GitHub release notes template](release.yml) and place the contents into your `.github/release.yml` file. | ||
- Customize our recommended template as a baseline and to configure how release notes are generated based on your project's issue labels. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@riverma Adding a picture(s) of how to this step generates release notes would be helpful for new users wanting to adopt SLIM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great suggestion @pogi7 - will do here 👍
Received a use case from the OPERA project regarding the following question: how do we safeguard that a super release will include the right versioned repositories? In other words, can this best practice guide PR help ensure that a particular build or release of many repositories doesn't lead to a situation where the wrong version of a particular repository's code is mixed up with another repository during a super release? |
So in terms of unity, we keep a changelog which is where most release notes comes from; thought this can still be very useful- there is a bit of a difference in what goes into a changelog and what we would describe at a system level release (which, in my opinion, is slightly different than these super releases. I can see "super releases" on a service area basis. The System releases, however, would be manually currated and de-jargoned for a slightly different audience. |
Recommend using AI/ML to process code commits in addition to commit and PR comments for generating release notes. There is significant benefit if AI/ML can extract release note information from code from that has not already been captured as a developer comment. |
Great idea! Will consider a prompt scenario to support this.
Rishi
…________________________________
From: stirlingalgermissen ***@***.***>
Sent: Thursday, July 11, 2024 11:56:26 AM
To: NASA-AMMOS/slim ***@***.***>
Cc: Verma, Rishi (US 398B) ***@***.***>; Mention ***@***.***>
Subject: [EXTERNAL] Re: [NASA-AMMOS/slim] Release Best Practices Guide (PR #149)
Recommend using AI/ML to process code commits in addition to commit and PR comments for generating release notes.
There is significant benefit if AI/ML can extract release note information from code from that has not already been captured as a developer comment.
—
Reply to this email directly, view it on GitHub<https://urldefense.us/v3/__https://github.com/NASA-AMMOS/slim/pull/149*issuecomment-2223672415__;Iw!!PvBDto6Hs4WbVuu7!OV9IQiv9KLKLbgpTfGg0oR_EHKxA5kY3SCIKx-TznvhjJwQ8kQ9pfBgfll6M8-MlZiGZDd36yHXoAYqwVWtp2yYNjvyOuA$>, or unsubscribe<https://urldefense.us/v3/__https://github.com/notifications/unsubscribe-auth/AAX36LWWZUZ6TI2YT4LGQYTZL3INVAVCNFSM6AAAAABFIE2S7SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRTGY3TENBRGU__;!!PvBDto6Hs4WbVuu7!OV9IQiv9KLKLbgpTfGg0oR_EHKxA5kY3SCIKx-TznvhjJwQ8kQ9pfBgfll6M8-MlZiGZDd36yHXoAYqwVWtp2ybCMkpDDA$>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Purpose
Proposed Changes
release.yml
file to help make release notes look good and standardized with categoriesIssues
Testing
release.yml
in action for the SLIM project here: https://github.com/riverma/slim/releases/tag/v1.4.0